[CLI-323] Added Supplier<T> defaults for getParsedOptionValue#229
Conversation
garydgregory
left a comment
There was a problem hiding this comment.
Hi @Claudenw
Thank you for your PR.
You'll need to add tests to cover the new feature.
|
I think I have this fixed now. |
|
|
||
| try { | ||
| return res == null ? defaultValue : (T) option.getConverter().apply(res); | ||
| return res == null ? defaultValue.get() : (T) option.getConverter().apply(res); |
There was a problem hiding this comment.
Hi @Claudenw
You're missing a test for when defaultValue is null.
|
|
||
| assertEquals(123, ((Number) cmd.getParsedOptionValue(optI)).intValue()); | ||
| assertEquals("foo", cmd.getParsedOptionValue(optF, "foo")); | ||
| assertEquals("foo", cmd.getParsedOptionValue(optF, ()-> "foo")); |
There was a problem hiding this comment.
If you know how to fix the checkstyle to look for this error I would appreciate it.
There was a problem hiding this comment.
If you know how to fix the checkstyle to look for this error I would appreciate it.
Hi @Claudenw
Done. Please rebase on git master.
garydgregory
left a comment
There was a problem hiding this comment.
@Claudenw
TY for your updates. Please see my comment.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #229 +/- ##
============================================
+ Coverage 91.90% 92.11% +0.21%
- Complexity 575 586 +11
============================================
Files 22 22
Lines 1247 1255 +8
Branches 210 212 +2
============================================
+ Hits 1146 1156 +10
+ Misses 63 61 -2
Partials 38 38 ☔ View full report in Codecov by Sentry. |
4f53cba to
c36c7f7
Compare
Fix for CLI-323 https://issues.apache.org/jira/browse/CLI-323